-
Notifications
You must be signed in to change notification settings - Fork 10.5k
HunyuanImage2.1: Implement Hunyuan Mixed APG #9882
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
comfy_extras/nodes_hunyuan.py
Outdated
|
|
||
|
|
||
| @classmethod | ||
| def IS_CHANGED(cls, model): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IS_CHANGED still needs to be removed, as per my last comment in the first PR. Node application is not on a per-step basis - always returning True from IS_CHANGED will break the proper node execution behavior. The only things in a per step basis would be the code that is being registered on the model patcher.
I think there may be an issue you are experiencing that is being incorrectly dealt with IS_CHANGED. Can you describe what was happening when you didn't have the IS_CHANGED? You may need to instead add a wrapper function to initialize/reset the dictionary you're trying to use to count steps at sample time.
comfy_extras/nodes_hunyuan.py
Outdated
| adaptive_projected_guidance_momentum=ocr_momentum | ||
| ) | ||
|
|
||
| current_step = {"step": 0} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Kosinkadink The reason we always return True in IS_CHANGED is that we need to track the dict current_step, which should be re-initialized before every run to reset current sampling step to 0. I find it hard to retrieve the current step index from the KSampler node so I track it by implementing this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Gotcha, in that case, what you are looking for is making an OUTER_SAMPLE wrapper, an example of which is done for the easycache_sample_wrapper in EasyCache nodes for core.
IS_CHANGED does not work how you would expect - returning True does NOT make it run every time, and is also not the way to make sure the code runs every time the sampler is ran; it is separate from the sampling process entirely. The return value of IS_CHANGED is like a 'fingerprint' - if this fingerprint is different than the last time the node instance was run, then the execution function of the node is permitted to run. If the fingerprint is the same, the node will NOT run and the cached value will be used instead. Always returning True from IS_CHANGED means the node will run ONLY ONCE the whole time it is on the graph (the fingerprint will always be 'True', and will match 'True' of the previous run). In V3 schema, this function got renamed to fingerprint_inputs to be more clear in its effects. The node running is completely separate from the sampling process.
What you'll want to do is make sure the dictionary you want to access gets reinitialized in the OUTER_SAMPLE wrapper. The function that OUTER_SAMPLE wraps runs every time sampling is initiated in ComfyUI, so it's the perfect place to reset the step value back to zero. This function also encapsulates sampling before CFG, so it won't get screwed up by the amount of conditioning being passed in.
|
@Kosinkadink Thanks for the tip! I’ve added the OUTER_SAMPLE wrapper. Could you please review this again? |
0959de8 to
fe7899d
Compare
Original
Updated
Prompt @Kosinkadink This is the comparison between our implementation and this PR. |
|
I found that if batch_size>1, it will not work properly File "/app/comfyui/comfy/samplers.py", line 978, in predict_noise |
percentage step to switch apg/cfg. Fix shape mismatch error for batchsize != 1
|
@Kosinkadink We adjusted the code to use percentage-based setting in the latest commit and will extract the refiner part separately into another PR. @wpdong0727 Thanks for your valuable feedback, we have fixed this bug in the latest commit. |
|
更新后的效果看起来极好,期待合并 |



HunyuanImage2.1: Implement Hunyuan Mixed APG
A workflow example:
HunyuanImage-2.1.json
cc @comfyanonymous